Skip to content

Conversation

@olaotesile
Copy link

Renamed BlobReader and BlobWriter to BlobFinder as suggested by @janwas in TODO. Also Updated associated files, tests, and CMakeLists.txt.

Addresses the TODO in io/blob_store.h by renaming the classes and files to BlobFinder/BlobWriter for better clarity. Verified file renames and updated CMake configuration.

Renamed BlobReader and BlobWriter to BlobFinder as suggested by @janwas in TODO. Also Updated associated files, tests, and CMakeLists.txt.
@google-cla
Copy link

google-cla bot commented Dec 26, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jan-wassenberg
Copy link
Member

Hello and happy holidays :) Thanks for tackling this. Would you mind retargeting this toward the dev branch? We are unable to merge into main directly.
Also, are you able to sign the CLA, which is also required before merging? https://cla.developers.google.com/

@olaotesile olaotesile changed the base branch from main to dev January 2, 2026 21:25
@olaotesile
Copy link
Author

Hi Jan,
Happy holidays to you too!

I've retargeted this to the dev branch, signed the CLA, and merged the latest dev changes to resolve the conflicts. Everything should be ready for review now.

I’m planning to focus heavily on low-level optimization and ML infrastructure this year, so I'll be keeping an eye on the issue tracker for more ways to contribute to gemma.cpp. Thanks!

@jan-wassenberg
Copy link
Member

Thanks! CLA looks good.
We have some remaining issues: model_store.h:32:10: fatal error: 'io/blob_store.h'.
Also, the Bazel build is failing because it also still has the old filename. There are quite a few remaining references to blob_store.h/cc. Maybe it's best if we do not rename the file, but only the classes and test, because blob_store does contain other classes such as BlobWriter. WDYT?

Nice, interesting topic! It's a fast moving field, lots going on :)

@olaotesile
Copy link
Author

Thanks for the catch! I really should have spotted those broken includes before pushing. My bad.

I've reverted the file names to blob_store and fixed the include in model_store.h, while keeping the BlobReader -> BlobFinder class rename. It should be good to go now!

And yeah, happy to be contributing to the 'lots' going on in the field! Hehe. Looking forward to more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants